Skip to content

Conversation

@GuanyiLi-Craig
Copy link
Contributor

  1. fix tests
  2. improve async
  3. add more unit tests

Copilot AI review requested due to automatic review settings December 31, 2025 16:26
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves async functionality, fixes tests, and adds comprehensive unit test coverage for assistant workflows. The changes focus on three main areas: refactoring quiescence detection logic to be simpler and more reliable, improving resource management with async context managers, and adding extensive unit tests for ReAct agents and Human-in-the-Loop (HITL) workflows.

  • Simplified quiescence logic by removing unnecessary work tracking state (_has_started, _total_committed)
  • Added proper async context manager usage for OpenAI and Claude clients to ensure resource cleanup
  • Introduced comprehensive unit tests using mock LLMs to test ReAct and HITL patterns without API calls

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
tests_integration/simple_llm_assistant/gemini_tool_example.py Relaxed content length assertions and added debug print
tests_integration/function_call_assistant/simple_gemini_function_call_assistant.py Updated Gemini model to version 2.5
tests/workflow/test_async_output_queue.py Improved test clarity by explicitly managing message publication/commitment lifecycle
tests/workflow/test_async_node_tracker.py Updated tests to reflect new quiescence logic (fresh tracker starts quiescent)
tests/tools/llms/test_openai_tool.py Fixed mock to properly implement async context manager protocol
tests/tools/llms/test_claude_tool.py Fixed mock to properly implement async context manager protocol
tests/assistants/test_assistant_mock_llm.py Added 2574 lines of comprehensive unit tests for ReAct and HITL patterns
grafi/workflows/impl/event_driven_workflow.py Added force stop call on node errors to prevent workflow hangs
grafi/workflows/impl/async_output_queue.py Added quiescence check during consume timeouts and changed error handling to propagate exceptions
grafi/workflows/impl/async_node_tracker.py Simplified quiescence logic, added reset_async method, removed work tracking state
grafi/topics/topic_base.py Added exception handling for condition evaluation failures
grafi/topics/queue_impl/in_mem_topic_event_queue.py Fixed lock usage in can_consume to ensure thread-safe state access
grafi/tools/llms/impl/openai_tool.py Wrapped AsyncClient usage in async context manager for proper cleanup
grafi/tools/llms/impl/claude_tool.py Wrapped AsyncAnthropic usage in async context manager for proper cleanup
grafi/tools/llms/impl/gemini_tool.py Updated default model to version 2.5
grafi/common/models/async_result.py Added producer task tracking and cancellation in aclose
grafi/common/containers/container.py Added thread-safe lazy initialization with double-checked locking

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

ayomaska18
ayomaska18 previously approved these changes Dec 31, 2025
@GuanyiLi-Craig GuanyiLi-Craig merged commit cdf4542 into main Dec 31, 2025
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants